Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update kzgbn254 function to work with eigenDA encoded blobs #2

Merged
merged 26 commits into from
Jun 5, 2024

Conversation

afkbyte
Copy link

@afkbyte afkbyte commented May 29, 2024

works with blobs encoded by Layr-Labs/eigenda#583

also uses some modified functions from the kzg library, diff can be found here:
https://github.com/Layr-Labs/rust-kzg-bn254/compare/master...afkbyte:rust-kzg-bn254:master?expand=1

when we recieve the data from the preimage it's already in evaluation formation not coefficient (has been ifftd before being dispersed) so we don't need to do a g1_ifft on the g1 SRS bc kzgcommitment = <C, IFFT(SRS)> = <IFFT(C), SRS>

@afkbyte afkbyte requested a review from epociask May 29, 2024 15:31
arbitrator/prover/src/kzgbn254.rs Outdated Show resolved Hide resolved
arbitrator/prover/src/kzgbn254.rs Outdated Show resolved Hide resolved
arbitrator/prover/src/utils.rs Outdated Show resolved Hide resolved
@teddyknox
Copy link
Collaborator

Awesome work @afkbyte. Can we rebase and clean up comments etc? Would love to get this merged.


// blob header is the first 32 bytes of the blob bytes
let blob_header = blob_bytes[..32].to_vec();
println!("blob header {:?}", blob_header);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another one.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


temp_buffer.reverse();
buffer.extend(temp_buffer);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't to_byte_array from the lib provide something similar or is this something more specific than that ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so it's a bit different, to_byte_array deserializes them in big endian but I need them in little endian

@@ -297,26 +297,18 @@ pub fn hash_preimage(preimage: &[u8], ty: PreimageType) -> Result<[u8; 32]> {
3000
).unwrap();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might want to use Prod numbers for these. In the lib, there is mainnet-data folder under tests. You can use those files to specify the input files.

Copy link
Collaborator

@epociask epociask left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mostly stylistic concerns around using idiomatic rust - otherwise builds and arbitrator tests are failing which should ideally be passing before merging

arbitrator/prover/src/kzgbn254.rs Outdated Show resolved Hide resolved
arbitrator/prover/src/kzgbn254.rs Outdated Show resolved Hide resolved
arbitrator/prover/src/kzgbn254.rs Outdated Show resolved Hide resolved
arbitrator/prover/src/kzgbn254.rs Outdated Show resolved Hide resolved
arbitrator/prover/src/kzgbn254.rs Outdated Show resolved Hide resolved
arbitrator/prover/src/kzgbn254.rs Outdated Show resolved Hide resolved
scripts/create-test-preimages.py Outdated Show resolved Hide resolved
arbitrator/prover/src/utils.rs Outdated Show resolved Hide resolved
@epociask
Copy link
Collaborator

epociask commented Jun 4, 2024

Also looks like we're introducing cargo.lock dependency changes which are causing jit compilations to fail in CI - lets fix that too before merging

https://github.com/Layr-Labs/nitro-private/actions/runs/9370515667/job/25797665060?pr=2

@afkbyte
Copy link
Author

afkbyte commented Jun 4, 2024

mostly stylistic concerns around using idiomatic rust - otherwise builds and arbitrator tests are failing which should ideally be passing before merging

stylistic issues addressed in ece9f22 and 028802a

python script comment removed in c505a1e

Copy link
Collaborator

@epociask epociask left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@afkbyte afkbyte merged commit a6dcc88 into develop Jun 5, 2024
6 checks passed
@epociask epociask deleted the afk/IFFTClient branch September 12, 2024 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants